Make it buildable on Windows with VS, and add windows Github Actions CI with vcpkg #75
Conversation
|
I'm not sure why not everything gets cached as I would have expected it for the vcpkg stuff, but anyway that can be solved later. |
adriweb
force-pushed
the
feature/ci-windows
branch
10 times, most recently
from
402a304 to
b88b521
Compare
|
The install folder is now uploaded as artifacts (for each build) :) |
adriweb
force-pushed
the
feature/ci-windows
branch
from
b88b521 to
6da204d
Compare
|
Compatibility with the MinGW cross-build should be OK now. |
adriweb
force-pushed
the
feature/ci-windows
branch
4 times, most recently
from
5aa6381 to
42525b0
Compare
adriweb
changed the title
adriweb
force-pushed
the
feature/ci-windows
branch
6 times, most recently
from
65dc215 to
04caffe
Compare
adriweb
force-pushed
the
feature/ci-windows
branch
from
04caffe to
472e62c
Compare
adriweb
force-pushed
the
feature/ci-windows
branch
3 times, most recently
from
1e42187 to
72c3bae
Compare
libticonv/trunk/src/iconv.c
Outdated
| @@ -61,10 +61,10 @@ TIEXPORT4 ticonv_iconv_t TICALL ticonv_iconv_open (const char *tocode, const cha | |||
| /* Convert at most *INBYTESLEFT bytes from *INBUF according to the | |||
| code conversion algorithm specified by CD and place up to | |||
| *OUTBYTESLEFT bytes in buffer at *OUTBUF. */ | |||
| TIEXPORT4 size_t TICALL ticonv_iconv (ticonv_iconv_t cd, char ** restrict inbuf, | |||
There was a problem hiding this comment.
Meh. This unconditionally reintroduces a previously long-standing known portability issue I fixed nearly 7 years ago in 933b4c6 .
Whatever the autotools build system does when configure.ac contains AC_C_RESTRICT - IIRC, something along the lines of replacing restrict by __restrict on a number of platforms - made libticonv cross-buildable for Win32 using the MinGW toolchain. I guess that the headers used by the MinGW toolchain - which must be drawing partially from MS headers - do not have the internal conflict with the Windows SDK you mention in the commit message ?
At least, this should be conditional on e.g. MSC_VER or the availability of the offending Windows SDK, right ?
There was a problem hiding this comment.
It looks like the following CMake test allows to conditionalize it:
if(NOT c_restrict IN_LIST CMAKE_C_COMPILE_FEATURES)
add_compile_definitions(restrict=__restrict)
endif()There was a problem hiding this comment.
Actually nevermind, it's a bit more involved, but it should be doable.
There was a problem hiding this comment.
Nevermind, I see no way of making it work. __restrict seems to be the way to use on clang, gcc, and MSVC.
I suggest that if anything else is required somewhere, a #define be made.
adriweb
force-pushed
the
feature/ci-windows
branch
4 times, most recently
from
b93a3a9 to
9322ef5
Compare
adriweb
force-pushed
the
feature/ci-windows
branch
from
9322ef5 to
04c0b77
Compare
|
I have integrated patches 1-3 and 5, and rebased experimental2 on top of those. It creates both conflicts for this PR and build failures on experimental2... |
|
Okay, I'll rebase and push again |
adriweb
force-pushed
the
feature/ci-windows
branch
from
04c0b77 to
bcc397e
Compare
|
I'm not surprised about the static build failures on the other branches, and the fact that they are fixed here. I did indeed fix quite a few CMake things recently on this branch. |
|
I've integrated 3 of the 5 patches at the front of the experimental2 branch (for now), in order to resolve the modify / delete conflicts caused by the torture_ticalcs.c -> torture_ticalcs.cc rename. I should dust off my cross-MinGW build procedure to test the result before forwarding the master branch. |
adriweb
force-pushed
the
feature/ci-windows
branch
from
bcc397e to
14d9b5a
Compare
|
I saw the same warning, but it was not promoted to an error. I guess we can fix that easily. Are there any other ones?
|
adriweb
force-pushed
the
feature/ci-windows
branch
2 times, most recently
from
449904b to
33cbaec
Compare
Note: we cannot simply do a -Drestrict=__restrict because of an internal conflict with the windows SDK
adriweb
force-pushed
the
feature/ci-windows
branch
from
33cbaec to
62201ce
Compare
|
I'll merge the remainder and perform some post-merge fixes. |
This branch is already rebased on top of the other one with macOS/Linux CI, btw.